chore: Remove unusued performance metrics system#762
Conversation
|
Failing check about missing tests can be ignored - simply removing dead code. |
fabioh8010
left a comment
There was a problem hiding this comment.
Two comments:
- Can we remove https://github.com/Expensify/react-native-onyx/blob/main/README.md#benchmarks too? It's useless and mentioning configs that don't even exist anymore
- I think we should still test this change against E/App, even if it's dead code removal. Can be something simple e.g. opening the app and making sure it works. Could you test and add test steps to
Manual Testsand screenshots too?
|
Got it, will do |
…x into chore/remove-unusued-metrics-system
MarioExpensify
left a comment
There was a problem hiding this comment.
Nice, thank you for the clean up
|
Hmmm, looks like we can't merge without the require-tests stage passing, let me check internally as this is just a removal of unused code. |
I believe it is safe to ignore it in this particular case. The workflow failed because Ive changed the files in /lib but no tests were modified. Metric system didnt have any tests so there is nothing to remove - thus failing check |
|
@MarioExpensify looks like this was merged without a test passing. Please add a note explaining why this was done and remove the |
|
Removed Emergency label, check this comment for reference. |
Details
Remove unused performance metrics system
Related Issues
Expensify/App#86418
Automated Tests
N/A - straight dead code removal
Manual Tests
Prerequisites:
In order to test this PR, you have to run E/App with this branch linked as the source of Onyx library
Open the App and see it works without any issues or crashes
Author Checklist
### Related Issuessection aboveTestssectiontoggleReportand notonIconClick)myBool && <MyComponent />.STYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)/** comment above it */thisproperly so there are no scoping issues (i.e. foronClick={this.submit}the methodthis.submitshould be bound tothisin the constructor)thisare necessary to be bound (i.e. avoidthis.submit = this.submit.bind(this);ifthis.submitis never passed to a component event handler likeonClick)Avataris modified, I verified thatAvataris working as expected in all cases)mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari
Web.mov